Skip to content

fix: limit offenses when voting in tally slashing mode by slashMaxPayloadSize#20683

Open
spypsy wants to merge 1 commit intomerge-train/spartanfrom
spy/tally-slasher-max-payload
Open

fix: limit offenses when voting in tally slashing mode by slashMaxPayloadSize#20683
spypsy wants to merge 1 commit intomerge-train/spartanfrom
spy/tally-slasher-max-payload

Conversation

@spypsy
Copy link
Member

@spypsy spypsy commented Feb 19, 2026

Fixes A-507

@spypsy spypsy marked this pull request as ready for review February 19, 2026 13:03
@spypsy spypsy marked this pull request as draft February 19, 2026 13:03
@spypsy spypsy marked this pull request as ready for review February 20, 2026 13:03
@AztecBot AztecBot force-pushed the spy/tally-slasher-max-payload branch from eaee53d to 31085e6 Compare February 20, 2026 13:10
@AztecBot AztecBot enabled auto-merge February 20, 2026 13:10
Copy link
Collaborator

@PhilWindle PhilWindle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks ok to me. @spalladino is this approach sufficient to at least fix the bug?

Copy link
Contributor

@spalladino spalladino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads up we're limiting the total number of offenses here, but IIRC the gas usage when slashing is dominated by the number of slashed parties, not individual offenses. We should trim the list of offenders rather than the list of offenses.

Also, note that here we're trimming the offenses we load from the store, but those then get expanded with always-slash offenses from config, so we could overrun the max size.

I think the right place to truncate is getSlashConsensusVotesFromOffenses, which is called immediately before computing the tally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants